Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve UX of namespace select filter #7783

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented May 25, 2023

closes #7590

NOTE:

This PR also includes performance improvements to the NamespaceSelectFilter in the form of using VirtualList as part of the new implementation.

Screen.Recording.2023-05-25.at.3.31.55.PM.mov

@Nokel81 Nokel81 added enhancement New feature or request area/ui labels May 25, 2023
@Nokel81 Nokel81 added this to the 6.6.0 milestone May 25, 2023
@Nokel81 Nokel81 requested a review from a team as a code owner May 25, 2023 19:33
@Nokel81 Nokel81 requested review from jansav and aleksfront and removed request for a team May 25, 2023 19:33
@Nokel81 Nokel81 force-pushed the improve-namespace-selector branch 2 times, most recently from 426d33e to 14bd0ef Compare May 26, 2023 14:44
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Nokel81 Nokel81 force-pushed the improve-namespace-selector branch from 5fe8b46 to 4a238e4 Compare June 1, 2023 12:43
@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2023

Conflicts have been resolved. A maintainer will review the pull request shortly.

Nokel81 added 10 commits June 1, 2023 09:44
- Add better support for thousands of namespaces

- Add support for mouse only multi-selection

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
…filter behaviour

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
…amespace

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 force-pushed the improve-namespace-selector branch from c30a7f5 to 6d3342e Compare June 1, 2023 13:44
Copy link
Contributor

@aleksfront aleksfront left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, this is a great improvement to the namespace selector! However, there are a few issues that I would like to address and suggest changes for, either immediately or in different pull requests.

To ensure intuitive understanding and ease of use, I propose the following improvements:

  • Remove the "namespace" icon as it doesn't serve a clear purpose before being clicked. It should be immediately apparent without any confusion.
  • Eliminate the need for advising tooltips. Depending on hints suggests a subpar user experience.
  • Replace the icons with more familiar checkboxes.
  • When clicking on a namespace name, only that particular namespace should be selected while others are deselected.
  • Clicking on the checkbox should add the corresponding namespace to the list.
  • Incorporate accessibility features that are currently missing.
  • The overall styling requires attention, including fine-tuning the colors for the light theme.

Please refer to the following images:

Elements to be removed:

elements to be removed

Replace icons with checkboxes:

replace icons with checkboxes

I recommend splitting these changes into multiple pull requests, with a special emphasis on highlighting the performance improvement aspect, which is currently of utmost importance.

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Jun 14, 2023

When clicking on a namespace name, only that particular namespace should be selected while others are deselected.

Clicking on the checkbox should add the corresponding namespace to the list.

Both of these are already implemented...

Eliminate the need for advising tooltips. Depending on hints suggests a subpar user experience.

I don't think we have a need for them, but I don't see how having hints are bad.

Remove the "namespace" icon as it doesn't serve a clear purpose before being clicked. It should be immediately apparent without any confusion.

I think if I instead make the icon non-focusable (and instead the text focuses) I think it should be fine to keep them. I think it adds a nice contrast between each individual namespace and the "All Namespaces" option.

Incorporate accessibility features that are currently missing.

This is rather vague, which ones do you mean exactly?

The overall styling requires attention, including fine-tuning the colors for the light theme.

Examples? I am already using the colour CSS variables from our themes.

Replace the icons with more familiar checkboxes.

I would like to get some more insight as to why this is better. Sure a checkbox by itself implies multiselection, but I think that there are other ways of implying multiselection, especially one that is "confirmed" immediately. Would having the + buttons only appear when hovering improve the UX you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lens freezes on namespace selection for cluster with 20k namespaces
2 participants